-
Notifications
You must be signed in to change notification settings - Fork 154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix and test derivative(x, 2x) #1078
Fix and test derivative(x, 2x) #1078
Conversation
It's a unityper type https://github.com/YingboMa/Unityper.jl So you need to check the EXPR type https://github.com/JuliaSymbolics/SymbolicUtils.jl/blob/d68e092a646ee746becc16fa5ec0e7e066a96f12/src/types.jl#L252 |
This breaks |
I am fine with having more errors, more safety. |
This also breaks differentiating with respect to an array of symbols (which we should presumably fix) and with respect to |
The crux of this issue is that we assume two expressions are constant with respect to eachother unless proven otherwise. julia> @variables x, y
julia> Symbolics.derivative(x, y)
0 This PR does not fix that issue, and does introduce errors which previously worked e.g. all of these results will become errors:
|
src/diff.jl
Outdated
@@ -31,8 +31,13 @@ julia> D3 = Differential(x)^3 # 3rd order differential operator | |||
""" | |||
struct Differential <: Operator | |||
"""The variable or expression to differentiate with respect to.""" | |||
x | |||
Differential(x) = new(value(x)) | |||
x::BasicSymbolic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't limit it to BasicSymbolic
src/diff.jl
Outdated
x::BasicSymbolic | ||
function Differential(x) | ||
vx = value(x) | ||
vx isa BasicSymbolic && SymbolicUtils.exprtype(vx) == SymbolicUtils.SYM || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should just check !istree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR uses unnecessary internal details.
Differential(x) = new(value(x)) | ||
function Differential(x) | ||
vx = value(x) | ||
istree(vx) && throw(ArgumentError("Cannot differentiate with respect to $vx")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also not quite correct. We frequently need to differentiate wrt variables with an independent variable like x(t)
.
Fixes #1077